Moving the FlowAdapters into the core jar as a solution to #424 #430
Conversation
|
Will you be able to build the flow adapter class with javac 9 while keeping the other class files as Java 6? |
|
@akarnokd Everything will be compiled with source and target 1.6, but FlowAdapters will only be linked/used in the build if compiled against jdk9+ (conditional sources). |
|
@reactive-streams/contributors Anyone? |
|
Sanity check the:
as I'm missing where the magic that does this in our build is here. If it indeed does so, this is ready, except the minor docs addition requested above. |
|
To be clear, if this can be done so that all |
|
It has to be done such that those are java 6 format. We do so in Akka where most classes (in the same jar) are JDK8 yet the Flow.* using ones are JDK9 compiled; the same has to be done here with regards to Java 6; which is why I'm asking if we confirmed this actually is happening now. |
|
ping @viktorklang can you confirm the result is really the expected one? |
There was a problem hiding this comment.
Figured it'll be quicker if I checked I guess.
It's all JDK6 format, which is fine. (and confirms my confusion that we did not do anything special to compile the java9 files in Java9 class format, but that's not really needed).
Class format is 50 everywhere, which is fine, the adapters project does not need to be compiled in format 52 after all, it only has to "touch" the JDK9 classes during compilation, and not compiling in runtime relates to the classes as well, not the format; we're okey here.
wip-424 u=!reactive-streams $> jenv local
9
wip-424 u=!reactive-streams $> java -version
java version "9"
Java(TM) SE Runtime Environment (build 9+181)
Java HotSpot(TM) 64-Bit Server VM (build 9+181, mixed mode)
wip-424 u=!reactive-streams $> ./gradlew install
INFO: ------------------ JDK9 classes detected ---------------------------------
INFO: Java 9 Flow API found; Including [tck-flow] & FlowAdapters in build.
INFO: --------------------------------------------------------------------------
BUILD SUCCESSFUL in 3s
32 actionable tasks: 4 executed, 28 up-to-date
wip-424 u=!reactive-streams $> javap -v api/build/classes/java/main/org/reactivestreams/Subscriber.class | grep version
minor version: 0
major version: 50
wip-424 u=!reactive-streams $> javap -v api/build/classes/java/main/org/reactivestreams/FlowAdapters.class | grep version
minor version: 0
major version: 50
wip-424 u=!reactive-streams $> echo "Java 6 uses major version 50."
Java 6 uses major version 50.
In other words LGTM
|
Thanks @ktoso—I didn’t see the previous messages due to a broken filter in
GMail.
👍
--
Cheers,
√
|
|
@reactive-streams/contributors Merging! |
|
Some users on Android + ProGuard have run into some issues due to the mixed RS jar file: ReactiveX/RxJava#6670. This would affect any library depending on RS 1.0.3+ under similar circumstances. Would the README.md of this repo a good place to document this case and the workarounds? |
|
Perhaps open a question on SO? Seems like a very specific issue related to a tool. (not finding a type which is not called in the codebase seems like a no-problem). |
Solves #424 by moving the FlowAdapters into the main jar and retires the flow-adapters artifact.
@reactive-streams/contributors Thoughts?